Repository navigation
fix: wipe secrets on every exit of softsign keygen, softsign import and yubihsm setup - #15
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three commands ended the process with
process::exitwhile key material was in memory, andprocess::exitruns no destructors, so the key was never wiped. This is the fix #14made for
yubihsm keys import, applied to them:softsign keygenandsoftsign importexit on a failed write only once the key is dropped.Import also keeps the secp256k1 bytes it writes in a buffer that wipes itself.
yubihsm setupreturns why it stopped, and exits only after the admin mnemonic, the rolepasswords and the wrap key have been dropped. That covers a declined erase, an undecodable
mnemonic and an HSM error.
--print-onlyno longer callsexit(0).password_file, itread the file again at the erase step, where a failed read ended the process with every
secret held.
mnemonic, the words it prints, and the entropy it derives the mnemonic from.
Exit codes and messages are unchanged. FORK.md row 14 now covers key import and setup, and
its note names what stays in memory anyway: the copies
yubihsmkeeps of every key setupinstalls, and the standard library's stdin and stdout buffers.
It also fixes a flaky test: a TCP test listens for its KMS on a port the OS assigns, and a UNIX
test KMS keeps its socket in its own directory under
/tmp. Test builds now have no warnings with anybackend alone.
Changes
4cbd624,4b012c7softsign keygenandsoftsign importexit on a failed write —0a98cb9,cac6f7dyubihsm setuponly after its secrets are dropped —7f6b8f3ed35a69bf5356eef9156622faab3,d565c708e740b9randdev-dependency —8ab6d0a,4a6e71f522ce0a159b1d8,a347687,be1865d,9525dd4Testing
softsign keygen,softsign importandyubihsm setup(on MockHsm, typing the mnemonic and the answer). Theones added first passed before the fixes, and all pass after.
run():one that exits in place kills the test process.
finishes; before the fix it exited with "couldn't read key from".
one. Before the fix it found it.
connection just used, which MockHsm accepts. That path changes only mechanically.
Test builds have no warnings with
yubihsm,yubihsm-server,softsign,ledgerorfortanixdsmalone.